Resp prepare #525
Resp prepare #525
Conversation
| @@ -629,10 +634,22 @@ def start(coding): | |||
| return | |||
|
|
|||
| def start(self, request): | |||
| warnings.warn('use .prepare(request) instead', DeprecationWarning) | |||
There was a problem hiding this comment.
Seems .start remain plain old function while .prepare is coroutine. Are they really interchangeable during deprecation period?
There was a problem hiding this comment.
They are do interchangeable with a note: .prepare() will process signal handing for response sending but .start() will not.
|
I do not like prepare coroutine, right now there is no real use case for coroutine signal. And database example is very good way to shoot yourself in the foot. You should realize, if you give users a gun they for sure shoot themselfs and they will blame you. Python is not fast, and adding more to hot execution path making aiohttp slower. But you are free to add this change, just be cearfull I do not want end up in situation like 0.12 and 0.15 |
|
The change is backward compatible (all existing code will continue working but for new code In practice users will
I'm not finding the way is elegant. |
|
Finally ready for merging. |
Signals support is very important for
aiohttp.web, I see strong request for the feature.We have a PR for that #439 (@alexsdutton, thanks a lot BTW for your work)
I insist signal handlers should be coroutines. Just signal callbacks are very subtle: we may want to access, say, database for modifying response headers (the main purpose for signals right now).
Keeping both synchronous and asynchronous signals just makes a mess -- signals should be coroutines. Period.
Thus I propose deprecation
StreamResponse.start(requiest)method in favor ofStreamResponse.prepare(request)coroutine (.begin()is an option but I feel.prepare()is better name).The change is backward compatible with limitation: old code still may use
resp.start(request)way but we will not apply signal handling for it.New code should use
.prepare()with benefit of signal handling.Any objections? @fafhrd91 @kxepal
TDB:
resp.start()and catch a deprecation warning). We should do it at least for keeping our test coverage level..start()deprecated and forcing to brand new.prepare()usage.